Skip to content

fix: stop spin button at exclusive bound#491

Closed
suhr25 wants to merge 4 commits into
open-telemetry:mainfrom
suhr25:fix/exclusive-min-max-number-input
Closed

fix: stop spin button at exclusive bound#491
suhr25 wants to merge 4 commits into
open-telemetry:mainfrom
suhr25:fix/exclusive-min-max-number-input

Conversation

@suhr25
Copy link
Copy Markdown
Contributor

@suhr25 suhr25 commented May 15, 2026

SUMMARY

Fixes an issue where NumberInputControl treated exclusiveMinimum / exclusiveMaximum as HTML min / max values. This caused the spin button to stop at values that were actually invalid and immediately failed validation.

FIX

Updated NumberInputControl to only use inclusive minimum and maximum values for the HTML input attributes. Since HTML number inputs treat min and max as inclusive bounds, using exclusive schema constraints was misleading the UI and allowing users to land on invalid values like 0 for fields that must be greater than 0. Added a regression test to ensure exclusive-only constraints no longer set HTML min/max attributes.

CHECKLIST

  • Reproduced issue locally
  • Applied fix in NumberInputControl
  • Added regression test
  • Verified tests pass locally

@suhr25
fix(config-builder): stop spin button at exclusive bound
2a87bc9 
Use only inclusive minimum/maximum for the HTML min/max attributes.
Exclusive bounds are already enforced by validateField; passing them
as the HTML attribute made the spin button stop at a value that
validation then rejected.

Signed-off-by: Suhrid Marwah <suhridmarwah07@gmail.com>
@suhr25 suhr25 requested review from a team as code owners May 15, 2026 09:46
@netlify
Copy link
Copy Markdown

netlify Bot commented May 15, 2026 •
edited
Loading

✅ Deploy Preview for otel-ecosystem-explorer ready!

Name Link
🔨 Latest commit 4a0b63d
🔍 Latest deploy log https://app.netlify.com/projects/otel-ecosystem-explorer/deploys/6a0ed8c4dc92090008452462
😎 Deploy Preview https://deploy-preview-491--otel-ecosystem-explorer.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@suhr25
Copy link
Copy Markdown
Contributor Author

suhr25 commented May 15, 2026 •
edited
Loading

Hi @vitorvasc @jaydeluca,
Kindly review.
Thanks!

@jaydeluca
Copy link
Copy Markdown
Member

jaydeluca commented May 18, 2026

@suhr25 can you provide steps on how to reproduce this issue? For example, which input do you see this problem occurring on?

@jaydeluca
Copy link
Copy Markdown
Member

jaydeluca commented May 20, 2026

in order to help keep track of what is ready for review, i'm going to put this into draft state. Feel free to open it back up when you have added reproduction steps.

If you are not able to actually reproduce this issue, then please close the PR.

@jaydeluca jaydeluca marked this pull request as draft May 20, 2026 10:12
@suhr25
Copy link
Copy Markdown
Contributor Author

suhr25 commented May 21, 2026

Thanks, I was able to reproduce this locally.

Steps:

  1. Open /java-agent/configuration/builder
  2. Enable Logger Provider → Processors → Add item → Batch
  3. Find "Max Queue Size", enter 1
  4. Click the spin-down arrow once

Before this PR, the spinner stops at 0, but tabbing away shows "Must be greater than 0".

Root cause:

// before
const min = constraints?.minimum ?? constraints?.exclusiveMinimum;

// after
const min = constraints?.minimum;

HTML min is inclusive, while exclusiveMinimum is not. A regression test is included in this PR.

@suhr25 suhr25 marked this pull request as ready for review May 21, 2026 10:25
@jaydeluca
Copy link
Copy Markdown
Member

jaydeluca commented May 21, 2026

Thanks for providing reproducer steps and investigating this.

After testing your branch against main, I don’t think this change improves the overall UX. Removing the constraint entirely means the spinner no longer has a lower bound, so users can decrement to -1, -100, etc. until validation runs on blur. In practice, that behavior seems worse than stopping at 0.

More generally, I want to reiterate feedback I’ve given on a few previous PRs: please try to focus issues and contributions on problems you actually encountered while using the site, or on cases that are likely to impact real users. Many edge cases are technically valid, but every issue and PR still takes maintainer time to investigate, validate, discuss, and maintain.

Right now, a number of the recent PRs feel very speculative or extremely low-impact, which makes it difficult for us to prioritize review time effectively.

Thanks again for taking the time to contribute, but I don’t think we’ll move forward with this change.

@jaydeluca jaydeluca closed this May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@suhr25 @jaydeluca